-
Notifications
You must be signed in to change notification settings - Fork 867
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Execute job in a domain and catch uncatched exception to mark job as failed #403
base: master
Are you sure you want to change the base?
Conversation
…failed. Uncatched exception make kue to stop, and make job stuck in unknown statement. Related to Automattic#391
Kue client codes can simply wrap their process in a domain or execute that with a promise based library. |
Client code have to be aware of the problem and the solution to wrap their process in a domain. And trust us, we are not. Moreover, if these exception are not catched, node will be terminate and all jobs will be stop without any warning. All day many job stuck due to this. I add a test, it do not caughtes all exception hiding them from client, it only catch uncatched exception. I do agree on client should protect their code with try catch, promise or err in callback, but we use many libraries, and not all of them are protected of uncatched error. I will just quote an article read on this topic:
Here a gist for the same patch as external patch, that have to be require() before using kue if you do not want jobs to be stuck due to bad library. https://gist.github.com/lchenay/2ca7651a0f57f50ce672 |
This is my fault not to mention this in documentation and suggest using domains/... to properly make your node.js process robust.
and the problem is the same, it hides uncaught exceptions from client's code, making their app very difficult to debug. This is the high most (application) layer code which should handle most of the exceptions, So I won't (i had the same patch ready for about 6 months in a private branch) add this until I find/am suggested proper solution to pass the exception to client code, or at least give them fully exception stack/context so that they can be informed of what is going wrong. |
Client are aware of failed job in this case:
|
Execute job in a domain and catch uncatched exception to mark job as failed
Uncatched exception make kue to stop, and make job stuck in unknown statement.
Related to #391